Make _observations.py use polars - #14169
Conversation
akustn
added
the
release-notes:improvement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14169 +/- ##
==========================================
- Coverage 91.92% 91.91% -0.02%
==========================================
Files 482 482
Lines 33498 33498
==========================================
- Hits 30794 30790 -4
- Misses 2704 2708 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
I took a look at the observations file in general and noticed that there are a lot repetetive strings that are used. |
SAKavli
left a comment
SAKavli
left a comment
There was a problem hiding this comment.
This is pretty much good to go!
I had some minor comments you can address.
| csv_file = pd.read_csv( | ||
| filename, | ||
| encoding="utf-8", | ||
| on_bad_lines="error", |
There was a problem hiding this comment.
Is this setting default in polars?
There was a problem hiding this comment.
Polars throws an error by default when there's malformed rows or parsing failures
| for row in csv_file.itertuples(index=True): | ||
| east_val = validate_float(str(row.EAST), "EAST") | ||
| north_val = validate_float(str(row.NORTH), "NORTH") | ||
| columns = set(csv_file.columns) |
There was a problem hiding this comment.
set(csv_file.columns)is called twice, could define this variable name farther up to avoid unnecessary operations
| date=str(row.DATE), | ||
| name=f"{observation_dict['name']}[{index}]", | ||
| well=str(row["WELL_NAME"]), | ||
| date=str(row["DATE"]), |
There was a problem hiding this comment.
Can you ensure it is tested that a file while doesn't contain the required rows raised an error nicely without traceback?
It is probably tested somewhere already, but nice to make sure.
There was a problem hiding this comment.
there is tests in test_observation_declaration.py covering this. :)
(test_that_missing_user_specified_property_raises_error and test_that_missing_columns_in_rft_observations_file_raises_error)
f290bf0 to
f84fb51
Compare
|
Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#84 |
f84fb51 to
9689d6a
Compare
|
Screenshot tests now pass. The baseline update PR equinor/ert-testdata#84 has been closed. |
9689d6a to
805f0fa
Compare
5 participants
Issue
Resolves #my_issue
Approach
Changing from pandas to polars in _observations.py.
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests')When applicable
merge screenshot-PR in ert-testdata before merging this PR.